Skip to content

[APPSEC-60218] Fix AWS API Gateway endpoints correlation HTTP span tags#967

Merged
dd-mergequeue[bot] merged 8 commits intomainfrom
flavien.darche/rfc-1081
Dec 18, 2025
Merged

[APPSEC-60218] Fix AWS API Gateway endpoints correlation HTTP span tags#967
dd-mergequeue[bot] merged 8 commits intomainfrom
flavien.darche/rfc-1081

Conversation

@e-n-0
Copy link
Copy Markdown
Member

@e-n-0 e-n-0 commented Dec 17, 2025 •
edited
Loading

Overview

Implementing changes from RFC 1081.

  • API Gateway REST/HTTP inferred spans now emit span.type as web
  • droped operation_name and apiname (in api gateway REST, HTTP and websocket)
  • Inferred spans now carry the API Gateway ARN in dd_resource_key
  • Appsec enablement and the json data from the service entry span are propagated into inferred and wrapped spans

Testing

  • Tested if dd_resource_key are correctly set for api gateway (HTTP/REST)
  • Tested if inferred spans correctly have the appsec data propagated (appsec enabled via entry span or via config)
  • Updated tests to remove operation_name occurence

@e-n-0 e-n-0 force-pushed the flavien.darche/rfc-1081 branch from c071b3d to 156c74e Compare December 17, 2025 14:12
@duncanista
Copy link
Copy Markdown
Contributor

duncanista commented Dec 17, 2025

Is the dd_resource_key exclusive to API Gateway or all inferred spans?

@e-n-0
Copy link
Copy Markdown
Member Author

e-n-0 commented Dec 17, 2025

No, it’s not exclusive to API Gateway. dd_resource_key is the CCRID, which represents the underlying cloud resource. For API Gateway inferred spans we want it to be the gateway ARN, and for Lambda spans it should be the Lambda function ARN, so it applies to all inferred spans that represent a cloud resource, not just API Gateway.

Right now, the tag already exists and appears to be set by the backend (since it is already always present and the value we set for API Gateway is being overwrited by the Lambda function ARN instead). The backend team will need to investigate this behavior later, but it shouldn’t block the current work.

@e-n-0 e-n-0 marked this pull request as ready for review December 17, 2025 15:41
@e-n-0 e-n-0 requested a review from a team as a code owner December 17, 2025 15:41
duncanista
duncanista reviewed Dec 17, 2025
View reviewed changes
Comment thread bottlecap/src/lifecycle/invocation/span_inferrer.rs Outdated

let identified_trigger = IdentifiedTrigger::from_value(payload_value);
let dd_resource_key =
Self::get_api_gateway_resource_key(&identified_trigger, &aws_config.region);
Copy link
Copy Markdown
Contributor

@duncanista duncanista Dec 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you make this a trait method for Triggers as opposed to make it API Gateway specific?

You don't need to implement it for every inferred span, but I want to make sure the SpanInferrer class doesn't do any trigger-specific logic unless required

Copy link
Copy Markdown
Member Author

@e-n-0 e-n-0 Dec 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Applied in 9acc70f and cddf7fc

duncanista
duncanista reviewed Dec 17, 2025
View reviewed changes
Comment thread bottlecap/src/lifecycle/invocation/span_inferrer.rs
);
s.meta.insert("span.kind".to_string(), "server".to_string());
let appsec_enabled = self.config.serverless_appsec_enabled;
propagate_appsec(appsec_enabled, invocation_span, s);
Copy link
Copy Markdown
Contributor

@duncanista duncanista Dec 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does the json need to be propagated up for the inferred spans?

Copy link
Copy Markdown
Contributor

@RomainMuller RomainMuller Dec 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is to address an issue raised by a customer who wants to be able to link attacks to API Gateway endpoints... Initially we wanted to do it backend-side, but we cannot guarantee the spans are in the same chunk so that was problematic... Copying the attack data to the inferred span makes this problem go away.

duncanista
duncanista reviewed Dec 17, 2025
View reviewed changes
Copy link
Copy Markdown
Contributor

@duncanista duncanista left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left a comment and a question!

Thanks for contributing 💯

@e-n-0
Copy link
Copy Markdown
Member Author

e-n-0 commented Dec 18, 2025

/merge

@dd-devflow-routing-codex
Copy link
Copy Markdown

dd-devflow-routing-codex Bot commented Dec 18, 2025 •
edited
Loading

View all feedbacks in Devflow UI.

2025-12-18 10:15:24 UTC ℹ️ Start processing command /merge

2025-12-18 10:15:28 UTC ℹ️ MergeQueue: pull request added to the queue

The expected merge time in main is approximately 24m (p90).

2025-12-18 10:16:44 UTC ℹ️ MergeQueue: This merge request was merged

@dd-mergequeue dd-mergequeue Bot merged commit c2ac1e0 into main Dec 18, 2025
48 checks passed
@dd-mergequeue dd-mergequeue Bot deleted the flavien.darche/rfc-1081 branch December 18, 2025 10:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@RomainMuller RomainMuller RomainMuller approved these changes

@duncanista duncanista duncanista approved these changes

Assignees

No one assigned

Labels

mergequeue-status: done

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@e-n-0 @duncanista @RomainMuller